feat: allow session config changes during a running turn - #3715
Conversation
10a5eee to
e425f73
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
I found a blocking issue that matches the pendingConfiguration path.
[P2] Pending configuration failure is not translated to a typed error
root-turn-coordinator.ts:1942 calls manager.applyPendingSessionConfiguration without catching SessionConfigurationTransitionError. The manager intentionally preserves pending and throws operation_unavailable, but prepareAdmittedTurn does not convert it via completedStart(operationUnavailable(...)). The exception bubbles out of sessionAdmission.run, so the client never receives the promised typed operation_unavailable payload and each retry re-enters the same unhandled rejection with pending permanently retained.
Fix: wrap the call and return completedStart(operationUnavailable(error.message)), consistent with other unavailable paths at line 511.
Head e425f73208. Checks on this head are currently 0 (no checks).
简体中文
该 pending 失败未转 typed error,客户端无法收到明确提示。|
Could you help clarify the product motivation for this pending configuration feature? I want to make sure the complexity is well justified:
简体中文能否请你补充一下这个 pending 配置特性的产品动机,以便确认复杂度是否值得:
|
|
Thanks for the review. I pushed commit 06052df with the requested fix. Root-Turn admission now catches SessionConfigurationTransitionError and returns the corresponding typed outcome (session_busy, operation_conflict, or operation_unavailable); unknown errors are still rethrown so the existing fail-stop behavior is preserved. Pending state remains persisted when application fails, and a regression test covers this path. I also fixed Runtime SessionManager summaries so pendingConfiguration remains visible to direct consumers. Product rationale: this is preparation for the next turn, not hot-swapping the active provider request. A common case is a long-running analysis, build, test, or tool-heavy turn where the user already knows the next request needs a different model, thinking level, or permission mode; waiting for the current turn adds avoidable latency, especially when a follow-up is queued. The pending selection is Host-owned Session state because the successor may be submitted by another client or admitted after reconnect/restart; renderer-local state could be lost or disagree with another client. The issue intentionally keeps the existing controls as the single configuration surface, without a separate current/next panel or persistent badge: the active AgentRun keeps its immutable snapshot, and the latest Host-authoritative value is consumed at the next root-Turn admission. This is an explicit product trade-off from the issue, not a claim based on a user study. Targeted verification on this head: SessionManager 225 passing; Root Turn Coordinator 50 passing. 中文摘要:已修复 pending 配置应用失败未返回 typed outcome 的问题;当前任务不会热切换,运行中的选择只在下一轮 root Turn admission 时生效,并由 Host 持久化以保证重连、重启和多客户端一致性。 |
Astro-Han
left a comment
There was a problem hiding this comment.
Update: the pending-configuration typed-error gap is now closed in this head.
root-turn-coordinator.ts:1942 now catches SessionConfigurationTransitionError and returns completedStart(operationUnavailable); session-manager.ts:6302 correctly projects pendingConfiguration. The new test returns a typed outcome when pending configuration cannot be applied covers the path. No remaining blocking issues.
简体中文
该阻断已在该头闭合。Hold pending feature necessity review per @astrohan
Summary
Fixes #3347.
Allow model, thinking-level, and permission-mode changes while a root Turn is running without mutating the active AgentRun.
Implementation
pendingConfigurationprojection with the Session header.Tests
npm test: 1403 passed.git diff --checkverification passed.